Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shared calendar invite to site #1642

Merged

Conversation

aj-stein-nist
Copy link
Contributor

@aj-stein-nist aj-stein-nist commented Feb 7, 2023

Committer Notes

DO NOT MERGE before #1638 is approved and merged in.

Current design looks as of last commit like the screenshot below. Will update as the commit evolves.

If approved and merged, this closes #1408.

image

All Submissions:

By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you included examples of how to use your new feature(s)?
  • Have you updated all OSCAL website and readme documentation affected by the changes you made? Changes to the OSCAL website can be made in the docs/content directory of your branch.

@aj-stein-nist aj-stein-nist self-assigned this Feb 7, 2023
@aj-stein-nist aj-stein-nist added the Scope: Website Issues targeted at the OSCAL project website. label Feb 7, 2023
@aj-stein-nist
Copy link
Contributor Author

An alternate approach, and I already got some negative feedback from one of the trusted members of the team (and that's ok! 😆) is embedding the iframe directly for easier access, but the style is jarring and ugly in my personal opinion. So the feeling is mutual. Comments and thoughts welcome.

image

@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Feb 7, 2023

Need to experiment more with some feedback I got.

  • Center both cards relative to the article
  • Stretch out both cards so they fill more of the page (maybe add some padding and margins?)
  • Add a bit more content explaining what each link is

@aj-stein-nist aj-stein-nist changed the title [WIP] Add shared calendar invite to site Add shared calendar invite to site Feb 13, 2023
@aj-stein-nist aj-stein-nist force-pushed the 1408-add-calendar-feed-to-website branch from 77f27f1 to 0c801c7 Compare February 13, 2023 17:26
@aj-stein-nist aj-stein-nist requested a review from a team February 13, 2023 17:26
@aj-stein-nist aj-stein-nist marked this pull request as ready for review February 13, 2023 17:28
@nikitawootten-nist
Copy link
Contributor

NB: For portability I think it would be a good idea to break out the calendar into its own shortcode so that it can be embedded into multiple pages.

That being said, premature optimization is the root of all evil and I might be suggesting this prematurely.

@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Feb 13, 2023

NB: For portability I think it would be a good idea to break out the calendar into its own shortcode so that it can be embedded into multiple pages.

That being said, premature optimization is the root of all evil and I might be suggesting this prematurely.

I think that is actually a wonderful idea, but we might want to backlog that and cross-ref with hugo-uswds since we would want it to be maximally reusable, right? I believe that is where almost all of our shortcode logic currently resides for the portability concerns you have.

That, or we can hoist it up later. Excellent point and thank you for the feedback, sir!

@aj-stein-nist
Copy link
Contributor Author

Aaaand it seems like the Azure version of Ubuntu package manage repos as configured by GHA to use in GHA runners has some problems and it is breaking one of the unrelated builds. Will have to circle back to this anyway.

@nikitawootten-nist
Copy link
Contributor

I think that is actually a wonderful idea, but we might want to backlog that and cross-ref with hugo-uswds since we would want it to be maximally reusable, right? I believe that is where almost all of our shortcode logic currently resides for the portability concerns you have.

Definitely ok with coming back to it, but as far as the hugo documentation suggests, you can define your own shortcodes outside of a theme in /layouts/shortcodes/<SHORTCODE>.html:

https://gohugo.io/templates/shortcode-templates/#shortcode-template-lookup-order

There is no need to hoist this into the theme itself.

Copy link
Contributor

@JustKuzya JustKuzya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked both links on PC. It works on both: the browser and the Outlook. Someone should check it on Mac's Outlook to make sure that URL works.

We probably should add an explanation that ICS can be opened as new to be kept as a separate calendar to allow users not to merge the meetings into their calendars if they so desire or advise them to use the HTTP link to online outlook standalone calendar.

Copy link
Contributor

@iMichaela iMichaela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the functionality and it appears that the iCalendar works as designed. Since there are 2 approvals already, you do not need mine to move forward.
But I will not Approve the PR because it will imply I agree with flooding my calendar/ people's calendars with meetings I/they do not need and which I/they need to manually delete. We have now only Feb meetings on the events calendar. I need to also understand the envisioned operational process as we will keep adding events to the calendar. Will I grab in 3-4 months from now all past and future events through this .ics file? Will we refresh the .ics file every month to gather only the events of that month and every member of the community will have to manually go and grab the .ics file load all meetings and then clean the ones they do not need/want?

@aj-stein-nist
Copy link
Contributor Author

aj-stein-nist commented Feb 17, 2023

I tested the functionality and it appears that the iCalendar works as designed.

But I will not Approve the PR because it will imply I agree with flooding my calendar/ people's calendars with meetings I/they do not need and which I/they need to manually delete.

I addressed your individual point in the specific comment above: if people wish to look in the web, configure the dedicated calendar or both, they get to control what they see and how they see it, more than only emails that send only to their personal calendar, and each change must be reviewed and accepted so an integrated client (an email and calendar app that coordinate, Outlook is popular but not all work this way) properly send the update to their integrated calendar. If they do not, as has happened at least 33% of the time for two standing public meetings a month, at least one, sometimes two or more, are confused as they are today.

We have now only Feb meetings on the events calendar. I need to also understand the envisioned operational process as we will keep adding events to the calendar. Will I grab in 3-4 months from now all past and future events through this .ics file? Will we refresh the .ics file every month to gather only the events of that month and every member of the community will have to manually go and grab the .ics file load all meetings and then clean the ones they do not need/want?

We have not defined a process, but I can envision we can come up with one in 15 minutes. That happens between #1638 and this PR. That is why this PR, in bold letters is addressed to the devs, but also specifically me, says verbatim:

DO NOT MERGE before #1638 is approved and merged in.

Let us know if you have additional questions, comments, or concerns.

@iMichaela
Copy link
Contributor

That is why this PR, in bold letters is addressed to the devs, but also specifically me, says verbatim:

DO NOT MERGE before #1638 is approved and merged in.

I did read the note, and also reviewed the ADR. Please disregard my comments. The devs do not need any help testing it on a Mac laptop with MS Office client, Calendar, and Safari since there are two that have Macs.

@aj-stein-nist
Copy link
Contributor Author

Added draft of SOP and integrated team feedback, can work on that out of band as AC met for this and dev work previously approved.

https://github.com/usnistgov/OSCAL/wiki/Public-Events-Calendar-Management

@aj-stein-nist aj-stein-nist merged commit e7a1918 into usnistgov:main Feb 28, 2023
@aj-stein-nist aj-stein-nist deleted the 1408-add-calendar-feed-to-website branch February 28, 2023 19:26
nikitawootten-nist pushed a commit that referenced this pull request Apr 10, 2023
* Enhance website with link and embedded calendar.

* Approach 1: set up cards and make the options clear.

* Switch from lists to only `div`s, fix spacing.

* Refactor close to PR feedback from Nikita.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Website Issues targeted at the OSCAL project website.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Add a Calendar Feed for NIST OSCAL Events to OSCAL Site?
4 participants